Skip to content

feat(execute): Mainnet marked tests#1511

Merged
marioevz merged 13 commits into
mainfrom
mainnet-marked-tests
Oct 9, 2025
Merged

feat(execute): Mainnet marked tests#1511
marioevz merged 13 commits into
mainfrom
mainnet-marked-tests

Conversation

@marioevz
Copy link
Copy Markdown
Member

@marioevz marioevz commented Apr 30, 2025 •
edited
Loading

🗒️ Description

Mark a subset of prague tests as mainnet-sanity tests.

The idea behind this PR is to:

  • Mark at least one test per EIP to run in mainnet after the fork activation in order to sign-off the fork.
  • The selected tests must be extremely simple and use a minimal amount of gas and preferably no Eth value.
  • The tests would be executed using uv run execute remote --fork=Prague -m mainnet ....
  • The tests can be displayed using uv run fill --fork=Prague -m mainnet --collect-only for reference.

🔗 Related Issues

✅ Checklist

  • All: Set appropriate labels for the changes.
  • All: Considered squashing commits to improve commit history.
  • All: Added an entry to CHANGELOG.md.
  • All: Considered updating the online docs in the ./docs/ directory.
  • Tests: All converted JSON/YML tests from ethereum/tests have been added to converted-ethereum-tests.txt.
  • Tests: A PR with removal of converted JSON/YML tests from ethereum/tests have been opened.
  • Tests: Included the type and version of evm t8n tool used to locally execute test cases: e.g., ref with commit hash or geth 1.13.1-stable-3f40e65.
  • Tests: Ran mkdocs serve locally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.

@spencer-tb
Copy link
Copy Markdown
Collaborator

spencer-tb commented Apr 30, 2025

Would it be better to add the mainnet marks on a per test case basis?

      pytest.param(
            Spec.G1ADD,
            Spec.G1 + Spec.INF_G1,
            Spec.G1,
            None,
            id="G1ADD",
            marks=[pytest.mark.mainnet]
        ),

@marioevz marioevz force-pushed the mainnet-marked-tests branch from 454978e to c47b047 Compare April 30, 2025 23:54
@danceratopz
Copy link
Copy Markdown
Member

danceratopz commented May 5, 2025

@marioevz I removed the EIP spec version checker from fill's and execute's pytest*.ini files to stop these tests causing noise for ethPandaOps whilst using this branch. I am addressing this change properly in another PR.

@danceratopz
Copy link
Copy Markdown
Member

danceratopz commented May 5, 2025

@marioevz I removed the EIP spec version checker from fill's and execute's pytest*.ini files to stop these tests causing noise for ethPandaOps whilst using this branch. I am addressing this change properly in another PR.

WIP -> #1537

@marioevz marioevz mentioned this pull request May 9, 2025
Closed
14 tasks
@spencer-tb spencer-tb added type:feat type: Feature scope:execute Scope: Changes to the execute command labels May 14, 2025
@marioevz marioevz force-pushed the mainnet-marked-tests branch from 869737e to 3274eb2 Compare September 12, 2025 18:16
@marioevz marioevz added scope:tests Scope: Changes EL client test cases in `./tests` and removed scope:execute Scope: Changes to the execute command labels Sep 12, 2025
@marioevz marioevz mentioned this pull request Sep 12, 2025
Closed
19 tasks
@spencer-tb
Copy link
Copy Markdown
Collaborator

spencer-tb commented Oct 3, 2025

Would it be better to add the mainnet marks on a per test case basis?

      pytest.param(
            Spec.G1ADD,
            Spec.G1 + Spec.INF_G1,
            Spec.G1,
            None,
            id="G1ADD",
            marks=[pytest.mark.mainnet]
        ),

I'm still wondering if the above is worthwhile. Alternatively we could make execute/fill ignore the mainnet marker by default?

@marioevz
Copy link
Copy Markdown
Member Author

marioevz commented Oct 8, 2025

Would it be better to add the mainnet marks on a per test case basis?

      pytest.param(
            Spec.G1ADD,
            Spec.G1 + Spec.INF_G1,
            Spec.G1,
            None,
            id="G1ADD",
            marks=[pytest.mark.mainnet]
        ),

I'm still wondering if the above is worthwhile. Alternatively we could make execute/fill ignore the mainnet marker by default?

The only thing I like from keeping them separate is that we can focus on optimizing these tests for gas in order for them to not consume a lot of real Eth when executed. E.g. let's say we mark a test as a mainnet test and afterwards, in a different PR, we inadvertently modify the test to cover a different case and it ends up using much more gas. It's unlikely but I like the separation.

We could indeed make it so that fill does not go through these tests, but execute I still see the value.

spencer-tb
spencer-tb approved these changes Oct 9, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@spencer-tb spencer-tb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Sounds good!

@marioevz marioevz merged commit cfd16b4 into main Oct 9, 2025
16 checks passed
@marioevz marioevz deleted the mainnet-marked-tests branch October 9, 2025 13:37
kclowes pushed a commit to kclowes/execution-spec-tests that referenced this pull request Oct 20, 2025
@marioevz @danceratopz
feat(execute): Mainnet marked tests (ethereum#1511)
a4f77bc 
* feat(plugins/shared): Add "mainnet" marker

* new(tests): EIP-2537 mainnet tests

* fix(tests): EIP-2537 file rename

* new(tests): EIP-2935 mainnet tests

* new(tests): EIP-7623 mainnet tests

* new(tests): EIP-6110 mainnet tests

* fix(tests): EIP-7623

* new(tests): EIP-7002,7251 mainnet tests

* new(tests): EIP-7702: Mainnet tests

* fix(tests): tox on EIP-7623

* fix(tests): EIP-7702, extra parametrization

* fix(tests): fix small copy-paste error that caused unknown fixture error

* fixup: Remove env from all tests

---------

Co-authored-by: danceratopz <danceratopz@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@spencer-tb spencer-tb spencer-tb approved these changes

Assignees

No one assigned

Labels

scope:tests Scope: Changes EL client test cases in `./tests` type:feat type: Feature

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@marioevz @spencer-tb @danceratopz